PARQUET-1383: Parquet tools should indicate UTC parameter for time/timestamp types #513
Conversation
| "TIMESTAMP_MICROS_STRINGIFIER", "yyyy-MM-dd'T'HH:mm:ss.SSS z", timeZone) { | ||
| @Override | ||
| public String stringify(long value) { | ||
| return super.stringify(value) + String.format("%03d", Math.abs(value % 1000)); |
There was a problem hiding this comment.
Does this work as intended? It seems to me that it appends the micros after the already stringified millis-timestamp, but that does not end in the millis, but in the time zone instead (yyyy-MM-dd'T'HH:mm:ss.SSS z).
I would expect this to print something like "2018-08-17T07:41:12.345 America/Los_Angeles678".
| } | ||
| }; | ||
|
|
||
| static PrimitiveStringifier createTimestampStringifier(final LogicalTypeAnnotation.TimeUnit timeUnit, final boolean isAdjustedToUTC) { |
There was a problem hiding this comment.
Since others may use this code as a reference for proper timestamp-handling, I think it would better to use the correct types (Instant for UTC-normalized and LocalDateTime for non-UTC-normalized) instead of just printing the timestamps correctly.
nandorKollar
changed the title
parquet-column/src/main/java/org/apache/parquet/schema/PrimitiveStringifier.java
Show resolved
Hide resolved
| PrimitiveStringifier stringifier = PrimitiveStringifier.TIMESTAMP_MILLIS_STRINGIFIER; | ||
|
|
||
| assertEquals("1970-01-01T00:00:00.000", stringifier.stringify(0l)); | ||
| assertEquals("1970-01-01T00:00:00.000+0000", PrimitiveStringifier.TIMESTAMP_MILLIS_UTC_STRINGIFIER.stringify(0l)); |
There was a problem hiding this comment.
Could you make this more consistent? Currently one stringifier is in a local variable, while the other is referenced directly.
3 participants
This pull request addresses two topics: